-
Notifications
You must be signed in to change notification settings - Fork 54
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix bug to dedicated flusher #174
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need some cosmetic change. Let me take it over.
src/flusher.h
Outdated
@@ -73,17 +73,20 @@ class FlusherQueue { | |||
|
|||
class Flusher : public WorkerBase { | |||
public: | |||
enum FlusherType { GENERIC = 0x0, ONLY_HANDLE_ASYNC = 0x1, ONLY_HANDLE_SYNC = 0x2 }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While I agree on GENERIC
for non-dedicated situation, but the other two need to be renamed. Actually both are async processing. I'd rather name them
enum FlusherType { GENERIC = 0x0, FLUSH_ON_DEMAND = 0x1, FLUSH_ON_CONDITION = 0x2 };
src/db_mgr.cc
Outdated
std::string t_name = flusher_type == Flusher::FlusherType::ONLY_HANDLE_ASYNC | ||
? "flusher_async_" + std::to_string(ii) | ||
: (flusher_type == Flusher::FlusherType::ONLY_HANDLE_SYNC | ||
? "flusher_sync_" + std::to_string(ii) | ||
: "flusher_generic_" + std::to_string(ii)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as below, I'd rather do flusher
, flusher_od
, flusher_oc
. Also using switch
will be better for readability.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with flusher_od
and flusher_oc
, but should we do flusher_gen
for GENERIC case? Because the worker invocation is prefix base. Though GENERIC and ON_DEMAND/ON_CONDITION is kind of mutual exclusive.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
No description provided.